Legion rouge#41
Closed
Dargon789 wants to merge 21 commits into
Closed
Conversation
…ated: IthacaAccount
Add CircleCI configuration file to set up a basic pipeline with a say-hello job and workflow CI: Add .circleci/config.yml to define a say-hello job that checks out the code and prints a greeting using the cimg/base Docker image Add a workflow to orchestrate the say-hello job under the CircleCI 2.1 engine
Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
CI: Update Forge test command to use --rerun and increase verbosity to -vvvvv Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
Enhance the CI test step to automatically rerun failed tests and increase verbosity in Forge. CI: Enable the --rerun flag for Forge tests to retry failures automatically Increase Forge test verbosity from -vvv to -vvvvv Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
Change Forge test invocation from "forge test --rerun -vvvvv" to "forge test -vvv" Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
* Update ci.yaml (#2) CI: Update Forge test command to use --rerun and increase verbosity to -vvvvv Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com> * Update ci.yaml (#4) Enhance the CI test step to automatically rerun failed tests and increase verbosity in Forge. CI: Enable the --rerun flag for Forge tests to retry failures automatically Increase Forge test verbosity from -vvv to -vvvvv Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com> * Update ci.yaml (#5) Change Forge test invocation from "forge test --rerun -vvvvv" to "forge test -vvv" Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com> * Update ci.yaml Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com> * Create CNAME * Revert "Merge branch 'master'" This reverts commit 6c02fbf, reversing changes made to a317ddb. --------- Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
Signed-off-by: Dargon789 <64915515+Dargon789@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAdds Merkle tree–based signature verification support to IthacaAccount, including a new wrapped-signature format, corresponding tests, and CI workflow tweaks to limit test scope, plus a version bump. Sequence diagram for merkle-based signature verification in IthacaAccountsequenceDiagram
actor ExternalCaller
participant IthacaAccount
participant MerkleProofLib
participant EfficientHashLib
participant ECDSA
ExternalCaller->>IthacaAccount: unwrapAndValidateSignature(digest, signature)
activate IthacaAccount
IthacaAccount->>IthacaAccount: check signature.length
alt signature.length == 65
IthacaAccount->>ECDSA: recoverCalldata(digest, signature)
ECDSA-->>IthacaAccount: signer
IthacaAccount-->>ExternalCaller: (signer == IthacaAccount, 0)
else wrapped signature
IthacaAccount->>IthacaAccount: parse keyHash, prehash, merkle flags via LibBytes
alt prehash flag set
IthacaAccount->>EfficientHashLib: sha2(digest)
EfficientHashLib-->>IthacaAccount: newDigest
IthacaAccount->>IthacaAccount: digest = newDigest
end
alt merkle flag set
IthacaAccount->>IthacaAccount: _verifyMerkleSig(digest, innerSignature)
activate IthacaAccount
IthacaAccount->>IthacaAccount: decode proof, root, rootSig from calldata
IthacaAccount->>MerkleProofLib: verifyCalldata(proof, root, digest)
MerkleProofLib-->>IthacaAccount: isLeafInTree
alt isLeafInTree
IthacaAccount->>IthacaAccount: unwrapAndValidateSignature(root, rootSig)
IthacaAccount-->>IthacaAccount: (isValid, keyHash)
IthacaAccount-->>ExternalCaller: (isValid, keyHash)
else
IthacaAccount-->>ExternalCaller: (false, 0)
end
else standard wrapped signature flow
IthacaAccount->>IthacaAccount: key = getKey(keyHash)
IthacaAccount->>IthacaAccount: validate innerSignature with key
IthacaAccount-->>ExternalCaller: (isValid, keyHash)
end
end
deactivate IthacaAccount
Updated class diagram for IthacaAccount merkle signature supportclassDiagram
class IthacaAccount {
+unwrapAndValidateSignature(bytes32 digest, bytes signature) bool isValid
+unwrapAndValidateSignature(bytes32 digest, bytes signature) bytes32 keyHash
-_verifyMerkleSig(bytes32 digest, bytes signature) bool isValid
-_verifyMerkleSig(bytes32 digest, bytes signature) bytes32 keyHash
+getKey(bytes32 keyHash) Key
+_hashTypedData(bytes32 structHash) bytes32
+_hashTypedDataSansChainId(bytes32 structHash) bytes32
+getEIP712NameAndVersion() string name
+getEIP712NameAndVersion() string version
<<EIP712>>
<<GuardedExecutor>>
}
class MerkleProofLib {
+verifyCalldata(bytes32[] proof, bytes32 root, bytes32 leaf) bool
}
class LibBytes {
+loadCalldata(bytes data, uint256 index) bytes32
+truncatedCalldata(bytes data, uint256 newLength) bytes
}
class EfficientHashLib {
+sha2(bytes32 input) bytes32
}
class ECDSA {
+recoverCalldata(bytes32 digest, bytes signature) address
}
class Key {
+address signer
+bool isValid
}
IthacaAccount --> MerkleProofLib : uses
IthacaAccount --> LibBytes : parses wrappedSignature
IthacaAccount --> EfficientHashLib : optionalPrehash
IthacaAccount --> ECDSA : eip191Recover
IthacaAccount --> Key : authorizationData
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! Ce PR introduit des capacités de vérification de signature Merkle au contrat intelligent Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Owner
Author
|
@Mergifyio refresh |
Owner
Author
|
@Mergifyio update |
✅ Pull request refreshed |
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_verifyMerkleSig, the inline assembly assumes a well‑formedabi.encode(bytes32[] proof, bytes32 root, bytes rootSig)payload with no minimal length or bounds checks; consider adding basic length/offset validation before reading from calldata to avoid undefined behavior on malformed or adversarial merkle signatures. - The new wrapped signature format adds a merkle flag byte but
unwrapAndValidateSignaturestill truncatessignaturebefore reading both the prehash and merkle flags; this relies onLibBytes.loadCalldataignoring the truncated length and may be brittle—consider reading the two flag bytes before truncating or documenting this invariant clearly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_verifyMerkleSig`, the inline assembly assumes a well‑formed `abi.encode(bytes32[] proof, bytes32 root, bytes rootSig)` payload with no minimal length or bounds checks; consider adding basic length/offset validation before reading from calldata to avoid undefined behavior on malformed or adversarial merkle signatures.
- The new wrapped signature format adds a merkle flag byte but `unwrapAndValidateSignature` still truncates `signature` before reading both the prehash and merkle flags; this relies on `LibBytes.loadCalldata` ignoring the truncated length and may be brittle—consider reading the two flag bytes before truncating or documenting this invariant clearly.
## Individual Comments
### Comment 1
<location path="src/IthacaAccount.sol" line_range="542-549" />
<code_context>
+ bool merkle;
unchecked {
- uint256 n = signature.length - 0x21;
+ uint256 n = signature.length - 0x22;
keyHash = LibBytes.loadCalldata(signature, n);
signature = LibBytes.truncatedCalldata(signature, n);
// Do the prehash if last byte is non-zero.
if (uint256(LibBytes.loadCalldata(signature, n + 1)) & 0xff != 0) {
digest = EfficientHashLib.sha2(digest); // `sha256(abi.encode(digest))`.
}
+ merkle = uint256(LibBytes.loadCalldata(signature, n + 2)) & 0xff != 0;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Flag bytes are read after truncating the signature, leading to out-of-bounds reads and incorrect offsets.
With the current layout (`abi.encode(bytes(innerSignature), bytes32(keyHash), bool(prehash), bool(merkle))`), the trailing fields are at:
- `keyHash`: `len - 34 .. len - 3`
- `prehash`: `len - 2`
- `merkle`: `len - 1`
You set `n = len - 34`, then truncate `signature` to length `n`, but still read at `n + 1` and `n + 2`. After truncation `signature.length == n`, so these reads are past the end of the slice and no longer match the documented layout.
Instead, compute all positions from the original `len` and read flags before truncating, e.g.:
```solidity
uint256 len = signature.length;
uint256 keyPos = len - 34;
uint256 prehashPos = len - 2;
uint256 merklePos = len - 1;
keyHash = LibBytes.loadCalldata(signature, keyPos);
bool prehash = uint256(LibBytes.loadCalldata(signature, prehashPos)) & 0xff != 0;
merkle = uint256(LibBytes.loadCalldata(signature, merklePos)) & 0xff != 0;
signature = LibBytes.truncatedCalldata(signature, keyPos);
```
This keeps offsets consistent and avoids out-of-bounds reads.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Merkle tree–based signatures, a significant enhancement to the account's verification capabilities. The implementation adds a new verification path in IthacaAccount.sol and extends the wrapped signature format with a merkle flag. The changes are well-supported by tests, including a new fuzz test for Merkle signature validation that covers various scenarios. Existing tests have been correctly updated to accommodate the new signature format. My review found a minor documentation issue in IthacaAccount.sol regarding the encoding of the wrapped signature, which I've commented on. Overall, the changes are robust and well-implemented.
This comment was marked as resolved.
This comment was marked as resolved.
Owner
Author
|
@Mergifyio rebase |
|
Owner
Author
|
@Mergifyio rebase |
1 similar comment
Owner
Author
|
@Mergifyio rebase |
☑️ Command
|
|
Owner
Author
|
@Mergifyio refresh |
1 similar comment
Owner
Author
|
@Mergifyio refresh |
☑️ Command
|
✅ Pull request refreshed |
Remove executable bit from deploy/execute_config.sh, deploy/verify_config.sh, and prep/check-bytecode-changes.js (mode 100755 → 100644). Update lib/forge-std submodule from commit c2cf7017d27c1d20e74ace4dacb6c5ce4bbbe899 to 07853315f998f94dc724e464b1bab1270888ee64. No other content changes.
Owner
Author
|
@Mergifyio refresh |
✅ Pull request refreshed |
auto-merge was automatically disabled
April 10, 2026 00:56
Pull request was closed
This was referenced Apr 11, 2026
Merged
Merged
Closed
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add Merkle tree–based signature verification support and adjust wrapped signature format, updating tests and CI accordingly.
New Features:
Enhancements:
CI:
Tests: